CI: require swift-testing-extensions 0.3.1 for WASM (groundwork for running tests) - #89
CI: require swift-testing-extensions 0.3.1 for WASM (groundwork for running tests)#89mansbernhardt wants to merge 1 commit into
Conversation
|
While not implicitly necessary, this surfaces the need to get the TestingExtensions package building cross-platform on WASM. Otherwise this situation will repeat itself over other repositories I use it in as a dependency. I use the dependency to test logic in various packages so I will eventually need it to be functional on WASM. However, it's a bit of chicken or the egg because I was waiting for a solution to get not just builds happening on CI but also unit testing for WASM. So maybe I can use this a template to add WASM testing for TestingExtensions itself. I'm adding a build job in orchetect/swift-testing-extensions#3, and once that goes green a unit test run can be added after. |
I'm not sure if this sheds any light, but in TestingExtension's CI pipeline I have to disable prebuilds otherwise there are compilation errors. I also recall that if you somehow intermix prebuilds and non-prebuilds in the derived data folder, it can really jam things up. Deleting DerivedData may be necessary to rule that part out. I'm not sure what would be different about WASM here but this may be a clue. |
|
Why is the commit from #88 duplicated here? It's not necessary for building WASM CI tests to run (but not pass), which was why I suggested branching from main so it would be a totally separate concern. |
84a151f to
81060f2
Compare
|
Apologies on the duplicated commit — that was my error, I branched from the #88 work rather than from Your prebuilds hunch was exactly right, and it is the whole blocker. The SwiftPM CLI equivalent of your With that flag the Two real errors were hiding underneath it:
To be clear about where this leaves things: I have moved it from one compile error to two different ones, not to a passing suite. But the diagnosis is solid and the prebuilts part is verified. On So that may need only a release and a version bump rather than new work. Happy to test it here if you cut one — I can point this branch at it and report back. |
|
I pushed TestingExtensions 0.3.1 earlier today with a passing build on WASM. You're probably building off your local resolved file which would be behind. SwiftTimecode's manifest can be updated to that version so it forces it. |
Groundwork for running `swift test` on the WASM jobs rather than only `swift build`. Not green yet — see the PR description. 0.3.0 imports `SWCompression` from the `#if canImport(Darwin)` / `#else` branch, so it fires on every non-Darwin platform including WASI, and `SWCompression` has no WebAssembly support. 0.3.1 narrows it to `#if os(Linux)`, which is correct — on wasm32-unknown-wasip1, `os(WASI)` is true and `os(Linux)` is false. Requiring 0.3.1 makes the resolution explicit rather than leaving it to whatever a consumer's Package.resolved happens to hold.
81060f2 to
9f8a7f6
Compare
|
That was it — thank you. Retested with 0.3.1 and the dependency kept, and the tooling side is fully clear:
So I have dropped the dependency removal entirely; this PR is now a one-line bump to What remains is 8 errors, all in one file — public func samplesValue(sampleRate: Int) -> Intwith tests using Which means this PR is gated on the consistency work in #88 rather than on anything CI-side. Once totals are Happy to do the samples part along with the rest whenever you have decided how you would like #88 staged. |
No worries, just wanted to point that out before more commits were added downstream here.
Ok, that's great. If the refactors for audio sample rate are addressed in #88 then we can either 1) merge this PR now and open a new PR later to add the actual WASM CI test run job, or 2) keep this PR open and after merging #88 to main you can pull-up main into this PR and add the WASM CI test job here. |
|
Correction, and apologies — I got this wrong.
My earlier claim was an artifact of my own output filtering. Macro-expansion diagnostics carry no So the remaining blocker for compiling tests on WASM is still The other half of the diagnosis stands, and that one I did verify directly: 0.3.0 imports |
|
Ok we can circle back to this after there is a chance to do more triage on the issue. Just for clarification - Swift Testing itself does actually work in a WASM unit test run? As in, a brand new empty Swift package with a test target and one |
|
With #88 merged down to main, do we want to pursue this further prior to a patch release? My rationale is that if a WASM unit test reveals any holes that need patching up it would be better to have them all passing before a release. |
|
My view: no — don't hold a patch release for this. What is left here is blocked on something neither of us controls. The 0.3.1 bump does its job (it forces a WASM-capable resolution rather than leaving it to whatever a consumer's Whereas #88 is worth shipping on its own: it closes a trap that aborts all So my suggestion is: merge this as the one-line bump whenever convenient, cut the patch release off Being transparent about the interest: we currently pin your fork branch by |
|
Totally agree, in that case. I will push a patch release. There is no rush for a WASM CI test run -- but if you do happen to work out the blocks at some point please let me know. The benefit would be broad however as I would then be able to implement it on multiple other repositories moving forward where I currently only have build jobs. |
|
As it so happens, I bumped the dependency version directly on I'm fine to leave this PR open if you prefer to resume at some point in the future with the discussion history in-place. But it's fine if we just close it and open a new PR in future when/if needed. |
|
Confirmed — For the record, so nothing is lost when it disappears: the two blockers for actually running the test suite on WASM are
Plus the four practical notes from my own WASM test lane, in case they save you time when you get to it: Happy to open a fresh PR for the CI job once the macro issue is understood. Thanks for the review throughout — the |
Groundwork for running
swift teston the WASM jobs rather than onlyswift build. Not green yet — but the remaining blocker is no longer tooling, and is described below.(Rewritten: this PR originally removed
TestingExtensionsfromSwiftTimecodeCoreTests. With 0.3.1 that is unnecessary, so the removal is gone and this is now a one-line version bump.)The two tooling blockers, both solved
1.
SWCompression— fixed by 0.3.1. In 0.3.0,Algorithm+DEFLATE.swiftimports it from the#if canImport(Darwin)/#elsebranch, so it fires on every non-Darwin platform including WASI. 0.3.1 narrows it to#if os(Linux), which is correct: onwasm32-unknown-wasip1,os(WASI)is true andos(Linux)is false (probed directly). This PR requires 0.3.1 so resolution is explicit rather than depending on what a consumer'sPackage.resolvedhappens to hold.2. The macro errors — your prebuilds hunch was exactly right. The SwiftPM CLI equivalent of
-IDEPackageEnablePrebuilts=NOis--disable-experimental-prebuilts:With it, both
@Test→ "global variable must be a compile-time constant to use @section attribute" and@Suite→ "'@const' value should be initialized with a compile-time value" disappear.What is left: 8 sites, one file, and it is an API question
With 0.3.1 + that flag, the entire suite compiles for wasm32 except 8 errors, all in
Tests/SwiftTimecodeCoreTests/Timecode/Source/Timecode Samples Tests.swift— 4 integer literals and 4Double→Intconversions that overflow.These are not test bugs. The API is:
and the tests use values like
4_147_200_000— 24 hours at 48 kHz. On a 32-bit platform that is simply unrepresentable, so audio sample counts are a fourth total-count domain in the same family as frames, subframes andFraction— and the one with the least headroom, since it overflows at ordinary limits rather than at a hypothetical future frame rate.So this PR is gated on the consistency work discussed in #88, not on CI configuration. Once totals are
Int64, adding the WASM test job here should be mechanical.Notes for whoever wires the job up
Four things that cost me time on my own WASM test lane:
swift build --build-testshas no per-target form, so it builds every test target including Apple-only ones. I narrow the manifest from an env var to the targets I want plus their transitive first-party deps.Bundle.mainand traps before running anything. Pass--testing-library swift-testing.--no-parallel— a trap kills the process, and only serial execution makes "last test to start" the actual culprit.-Xlinker -z -Xlinker stack-size=…also reaches the host link of the macro plugins, and macOSldrejects it.